Skip to content

Add support for IPython magic#44

Merged
rbpatt2019 merged 14 commits into
mainfrom
fix043
May 21, 2022
Merged

Add support for IPython magic#44
rbpatt2019 merged 14 commits into
mainfrom
fix043

Conversation

@rbpatt2019

Copy link
Copy Markdown
Collaborator
  • build(pyproject.toml): add ipython support
  • feat(pyprql/magic): adds ipython magic
  • feat(magic/prql.py): rough working functionality
  • feat(pyproject.toml): add duckdb-engine
  • fix(magic/prql.py): change default configurations
  • fix(magic/prql.py): fix printing of results
  • docs(*): document magic

rbpatt2019 added 7 commits May 6, 2022 06:40
Adds ipython-sql and ipython as dependencies. The goal is to wrap the former and pass it parsed prql.
This is a minimal working example. It will fail in many cases, namely when used as a line magic where arguments are passed.
Major changes still required. Currently assumes that line magic will only be used to set up connection strings and options. Additionally, cell magic only returns correct results if the results are passed to a local variable.
Allows magic to use duckdb connection strings.
Defaults autopandas to true and displayconn to false.
By providing an autoview parameter and manually specifying `_` as the return variable, we can ensure that results are always accessible and always printable.
Provides full Sphinx documentation for IPython/Jupyter Magic.
Applies encessary lints to the file, while also updating format to run on Python 3.10/
Bring pyproject and lock file into alignment and update cache number.
Use of a local pre-release version of poetry had caused divergent formats in the lockfiles. This should bring those back in line.

@max-sixty max-sixty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job @rbpatt2019 !

Comment thread pyprql/magic/README.md
In [7]: %config PRQLMagic.autoview = False
```

[ipysql]: https://github.com/catherinedevlin/ipython-sql

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually hadn't seen this sort of link in markdown before — it's great!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a big fan of them. They make the document so much more readable in plain text, and allow you to re-use hyperlinks.

Comment thread pyprql/magic/prql.py Outdated
1. displaycon is set to ``False``.

Additionally,
to working around some quirky behaviour,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
to working around some quirky behaviour,
to work around some quirky behaviour,

Comment thread pyprql/magic/prql.py Outdated
The variables local to the running IPython shell.
"""
# If cell is occupied, it must be parsed to SQL
if cell != "":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit: arguably more idiomatic, although your call)

Suggested change
if cell != "":
if cell:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to lie, some how in years of coding Python, I had never picked that empty strings were False-y! All for this change

Comment thread pyprql/magic/prql.py Outdated

# If cell is occupied and line is empty,
# we artificially populate line to ensure a return value.
if cell != "" and line == "":

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if cell != "" and line == "":
if cell and not line:

Comment thread pyprql/magic/README.md
@@ -0,0 +1,184 @@
# Using the IPython and Jupyter Magic

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great docs! Maybe we can link to these from https://lang.prql.builders/ when they're live

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They'll go live (for the magic) once the PR is merged. The full docs live at https://pyprql.readthedocs.io/en/latest/# and are already live.

Comment thread pyproject.toml Outdated
{version = "^1.22.3", python = "^3.8"}
]
prql-python = "^0"
ipython = "7.33.0"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite restrictive — do we want ">=7"? Otherwise it's going to downgrade everyone who has ipython 8 installed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember why I limited this initially...I think since we support python 3.7, and IPython > 8 requires python 3.8 or greater. But there are definitely better ways to specify this. I'll see what we can do

Comment thread pyprql/magic/README.md Outdated
that's as simple as:

```shell
pip install PyPRQL

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had thought conventions were to have lowercase. Both seem to work though

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convention is lowercase. This is me just blindly accepting autocomplete :p

Comment thread pyprql/magic/README.md
```

Now,
the output of the query is saved to `results`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very clearly explained, again great docs!

Comment thread .github/workflows/cicd.yaml
Comment thread pyprql/magic/prql.py
if "<<" in line:
print(local_ns[line.split()[0]])
else:
print(local_ns["_"])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V impressive how brief the code is here! Nice work.

@max-sixty

max-sixty commented May 18, 2022

Copy link
Copy Markdown
Member

FYI I get this error, but it might well be my dependencies. Could you confirm it works for you @rbpatt2019 ?

image

In text form:

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Input In [2], in <cell line: 1>()
----> 1 get_ipython().run_line_magic('load_ext', 'pyprql.magic')

File /opt/homebrew/lib/python3.9/site-packages/IPython/core/interactiveshell.py:2304, in InteractiveShell.run_line_magic(self, magic_name, line, _stack_depth)
   2302     kwargs['local_ns'] = self.get_local_scope(stack_depth)
   2303 with self.builtin_trap:
-> 2304     result = fn(*args, **kwargs)
   2305 return result

File /opt/homebrew/lib/python3.9/site-packages/IPython/core/magics/extension.py:33, in ExtensionMagics.load_ext(self, module_str)
     31 if not module_str:
     32     raise UsageError('Missing module name.')
---> 33 res = self.shell.extension_manager.load_extension(module_str)
     35 if res == 'already loaded':
     36     print("The %s extension is already loaded. To reload it, use:" % module_str)

File /opt/homebrew/lib/python3.9/site-packages/IPython/core/extensions.py:76, in ExtensionManager.load_extension(self, module_str)
     69 """Load an IPython extension by its module name.
     70 
     71 Returns the string "already loaded" if the extension is already loaded,
     72 "no load function" if the module doesn't have a load_ipython_extension
     73 function, or None if it succeeded.
     74 """
     75 try:
---> 76     return self._load_extension(module_str)
     77 except ModuleNotFoundError:
     78     if module_str in BUILTINS_EXTS:

File /opt/homebrew/lib/python3.9/site-packages/IPython/core/extensions.py:93, in ExtensionManager._load_extension(self, module_str)
     91     with prepended_to_syspath(self.ipython_extension_dir):
     92         mod = import_module(module_str)
---> 93         if mod.__file__.startswith(self.ipython_extension_dir):
     94             print(("Loading extensions from {dir} is deprecated. "
     95                    "We recommend managing extensions like any "
     96                    "other Python packages, in site-packages.").format(
     97                   dir=compress_user(self.ipython_extension_dir)))
     98 mod = sys.modules[module_str]

AttributeError: 'NoneType' object has no attribute 'startswith'

Comment thread pyprql/magic/README.md Outdated

## Implementation

This is a this wrapper around the fantastic

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small typo: "thin wrapper"

Comment thread pyprql/magic/README.md Outdated

:::{Important}
This is one area where we differe from `IPython-sql`.
We only support parasing PRQL as a cell magic,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "parsing PRQL"

Comment thread pyprql/magic/README.md Outdated
:::{Important}
This is one area where we differe from `IPython-sql`.
We only support parasing PRQL as a cell magic,
not as a line magic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this as in line 41 above you seem to have a line magic example, or is that passing a parameter to the prql magic which is then used in the cell magics further on?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we do have a line magic, but it can only be used for connection strings and parameters. In the original Python-sql, line magic sql is valid, like this %sql select * from table. But they have quite an extensive parser to figure out whether the line magic is a connection string, flags, or sql. To the first order of approximation, they achieve this by checking to see if the line passed looks like SQL. So, if we wanted this functionality, we'd have to re-implement their parser, look for PRQL, parse the PRQL to SQL, re-assemble the line, then pass it to the magic. This seemed a bit of a faf, so for the momentI've limited the line magic to parameters and connections strings. I'll clarify in the docs!

Expands on our different uses of line and cell magic relative to IPython-SQL, and also corrects a few minor typos.
Empty strings are falsey while non empty strings are truth, so we can check them directly.
Allows those running more recent versions of Python to use more recent versions of IPython.
@rbpatt2019

Copy link
Copy Markdown
Collaborator Author

Also @max-sixty I can't reproduce the error using the most recent Jupyter notebook. I've updated the Python versions - let me know if the error still persists.

@qharlie

qharlie commented May 20, 2022

Copy link
Copy Markdown
Contributor

All of this is working great for me! I might take a stab at the line magic for inline queries after this is merged.

Also holy shiz everything is so fast!

Comment thread pyprql/magic/README.md Outdated
If you connect to an existing SQL database,
then all the tables normally there will be accessible.

One thing we don't support that `IPython-SQL` does is passing PRQL directly to a line magic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
One thing we don't support that `IPython-SQL` does is passing PRQL directly to a line magic.
One thing we don't support that `IPython-SQL` does is passing a PRQL query directly to a line magic.

Comment thread pyprql/magic/README.md Outdated
and it may be added as a feature in a future release.

:::{Warning}
This is one area where we differe from `IPython-sql`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This is one area where we differe from `IPython-sql`.
This is one area where we differ from `IPython-sql`.

@max-sixty

Copy link
Copy Markdown
Member

Also @max-sixty I can't reproduce the error using the most recent Jupyter notebook. I've updated the Python versions - let me know if the error still persists.

I think I had installed it incorrectly or the wrong branch, I can't repro my error, at least on the current version. Mea culpa

@max-sixty

Copy link
Copy Markdown
Member

This is very cool indeed.

I def think we should merge! Let's also link to these docs from the Jupyter link here (and everywhere else we're listing general resources). I'm happy to do that.

One thing that would make it easier for newcomers, in the next version of the docs, is to have an example that's possible to follow along with, given that many of the primitives are new — e.g. the magics, the duckdb:///:memory:, the --persist, etc — even aside from PRQL itself. I realize writing docs is often less enjoyable so I'm happy to help here if needed.

Here's an example which is not a good example per se, but is possible to follow along with — we can change the df creation to be something more interesting / highlighs more of prql's features https://gist.github.com/max-sixty/dfb1521f326742358d5b0bdf4b1c9698

@rbpatt2019 rbpatt2019 merged commit c6784fe into main May 21, 2022
@rbpatt2019 rbpatt2019 deleted the fix043 branch May 21, 2022 06:19
@max-sixty

Copy link
Copy Markdown
Member

Congrats! I'm v excited about this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants